Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

openapi3: allow Extensions next to $ref in SchemaRef #901

Merged
merged 1 commit into from
Jun 24, 2024
Merged

openapi3: allow Extensions next to $ref in SchemaRef #901

merged 1 commit into from
Jun 24, 2024

Conversation

paulmach
Copy link
Contributor

@paulmach paulmach commented Jan 30, 2024 •
edited
Loading

See #900 for full details and use-case. If this approach is agreed upon, I can extend to all *Refs (e.g. CallbackRef, ExampleRef, ResponseRef, etc.)

Captures x-order into SchemaRef.Extensions

type: object
properties:
 id:
   $ref: "#/components/schemas/id"
   x-order: 1
 name:
   type: string

Only extensions (those starting with 'x-') are captured. This is because extra attributes are technically not allowed by the spec, but also to maintain the way JSONLookup works. For example:

schemas:
  id:
    type: integer
    x-order: 2
  thing:
    type: object
    properties:
     thing_id:
       $ref: "#/components/schemas/id"
       type: string
       x-order: 1
     name:
       type: string

Doesn't change: thingIDRef.JSONLookup("type") will continue to return "integer" as overriding the type next to the ref is not allowed and bad practice, in my opinion. There could be a use-case for stuff like nullable, but for now I would rather not change the behavior.

Does change: thingIDRef.JSONLookup("x-order"). Today, 2, as it goes back to the $ref definition. After this change it'll be 1 (value next to the $ref).

Does change: Validate() behavior. It's now okay, by default, to have extensions next to the $ref. To change this behavior, use the ProhibitExtensionsWithRef option.

@paulmach paulmach mentioned this pull request Jan 30, 2024
Closed
@paulmach
Copy link
Contributor Author

Oh snap, looks like refs.go is auto generated. Please see if you like this change, then I make the change to refs.sh.

fenollp
fenollp reviewed Jan 31, 2024
View reviewed changes
openapi3/refs.go Outdated Show resolved Hide resolved
fenollp
fenollp reviewed Jan 31, 2024
View reviewed changes
openapi3/refs_test.go Outdated Show resolved Hide resolved
fenollp
fenollp requested changes Jan 31, 2024
View reviewed changes
openapi3/refs_test.go Outdated Show resolved Hide resolved
@paulmach
Copy link
Contributor Author

paulmach commented Feb 1, 2024

I made the following changes:

  • moved tests in ref_test.go into refs_issue222_test.go and refs_issue247_test.go
  • added validation options AllowExtensionsWithRef (default) and ProhibitExtensionsWithRef
  • updated refs.sh to generate the code
  • add refs_test.sh to generate tests
  • reran docs.sh and added refs_test.sh as part of the github workflow.

I think the only "question" is if ref.Extensions should capture ^x- fields or all extra fields. As mentioned above, there could be some impact for "weird" schemas if we capture all fields. I propose to only capture ^x- fields for now as more backwards compatible. We can always change it based on feedback.

@fenollp
Copy link
Collaborator

fenollp commented Feb 1, 2024

This is great work thank you!

WRT non x-.. in Extensions you're right: let's just keep the x-.. keys in here for now anyway

@fenollp fenollp linked an issue Feb 2, 2024 that may be closed by this pull request
v3: allow extensions next to $ref in SchemaRef #900
Closed
@paulmach paulmach requested a review from fenollp February 7, 2024 01:03
@paulmach
Copy link
Contributor Author

I rebased on the latest master. Hoping to get this into the next release.

@paulmach
Copy link
Contributor Author

paulmach commented Jun 9, 2024

Hi @fenollp, I rebased again on the latest master. Hoping to get this into the next release.

@paulmach paulmach mentioned this pull request Jun 10, 2024
Closed
@paulmach paulmach force-pushed the schemaref branch 2 times, most recently from ba91e23 to 63ca84c Compare June 23, 2024 05:26
fenollp
fenollp approved these changes Jun 24, 2024
View reviewed changes
openapi3/validation_options.go
// error if extensions (fields starting with 'x-') are found as
// siblings for $ref fields. Non-extension fields are prohibited
// unless allowed explicitly with the AllowExtraSiblingFields option.
func ProhibitExtensionsWithRef() ValidationOption {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mind renaming to ˋDisallow..ˋ? The other options in there start with Allow.. or Disallow.. (well, most)

openapi3/refs.go Outdated Show resolved Hide resolved
@fenollp fenollp merged commit 57624b3 into getkin:master Jun 24, 2024
5 checks passed
@paulmach paulmach mentioned this pull request Jul 13, 2024
Merged
@irees irees mentioned this pull request Aug 24, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fenollp fenollp fenollp approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

v3: allow extensions next to $ref in SchemaRef
2 participants
@paulmach @fenollp